Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Cleaning up files after cli move #30

Merged
merged 3 commits into from
May 9, 2017
Merged

Cleaning up files after cli move #30

merged 3 commits into from
May 9, 2017

Conversation

simonwalsh
Copy link
Contributor

Just cleaning up the repo of the files that will reside in the project folder instead of the tool

@simonwalsh simonwalsh self-assigned this May 8, 2017
@simonwalsh simonwalsh requested a review from nddery May 8, 2017 21:36
@simonwalsh simonwalsh changed the base branch from master to base-webpack-assets-pipeline May 8, 2017 21:36
@simonwalsh
Copy link
Contributor Author

See anything else @nddery ?

@simonwalsh
Copy link
Contributor Author

Also, I encountered the issue where I need to install all the dependencies of the CLI tool within the child project to be able to run the foobarify-scripts commands... Did you have to do this as well? I feel it is just a matter of pointing to the root of the tool instead of the root of the project's node modules... But maybe it is because we are using link and there is not a real dependency tree there. Unsure!

nddery
nddery approved these changes May 8, 2017
View reviewed changes
Copy link
Contributor

@nddery nddery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool - thanks for the .gitignore stuff too - and yes, had forgot about the dependencies.. I also think it's because we've used link and the package isn't really in the node_modules directory (when we'd yarn add foobarify it would then also install it's own dependencies so I'm pretty sure this is "normal" right now)..

@simonwalsh
Copy link
Contributor Author

Perfect, cool! Lemme make sure it runs when duplicating the dependencies into the project and we can figure out the rest once it is a proper npm package... My feeling though is that there has to be a way to do this before going public... Otherwise people would never be able to test stuff before that moment haha...

The answer probably lies somewhere in there https://docs.npmjs.com/private-modules/intro

@nddery
Copy link
Contributor

nddery commented May 8, 2017

So it indeed doesn't install the package - yarnpkg/yarn#2914. This yalc tool could come in handy if we don't want to install foobarify deps into the tests projects.

@simonwalsh
Copy link
Contributor Author

Or from the looks of it, we use npm to link maybe? I will play around with this tomorrow!

@simonwalsh
Copy link
Contributor Author

npm link does not work any better

@simonwalsh simonwalsh merged commit 199b154 into base-webpack-assets-pipeline May 9, 2017
@simonwalsh simonwalsh deleted the chore/cleaning-up-files-after-cli-move branch May 9, 2017 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nddery nddery nddery approved these changes

Assignees

@simonwalsh simonwalsh

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@simonwalsh @nddery